Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Theme JSON resolver: read theme.json files from the styles/ folder only once #62638

Merged
merged 1 commit into from
Jun 18, 2024
Merged

Theme JSON resolver: read theme.json files from the styles/ folder only once #62638

merged 1 commit into from
Jun 18, 2024

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Jun 18, 2024

What?

Cache reading theme.json files from styles/ folder.

Syncs the following Core patch to Gutenberg:

Why?

From WordPress/wordpress-develop#6843:

I've realized that, unlike the theme.json files defined by core & theme, we don't cache these — hence we end up reading them a few times from the filesystem.

How?

From WordPress/wordpress-develop#6843:

By using the existing read_json_file function instead of using wp_json_file_decode directly.

Testing Instructions

From WordPress/wordpress-develop#6843:

  1. Verify the theme style variations are working as expected.

Go to "Site Editor > Styles" and apply one of them. Verify the changes are reflected in the frontend.

Do the same from the global styles sidebar in the site editor.

  1. Verify the block style variations defined via theme.json are working as expected.

Create a partial.json file within the styles/ folder with the following contents:

{
        "$schema": "https://schemas.wp.org/trunk/theme.json",
        "version": 2,
        "title": "Partial",
        "blockTypes": [ "core/group" ],
        "styles": {
                "color": {
                        "background": "aliceblue"
                }
        }
}

Go to any editor, add a group block, and verify there is a "Partial" style variation (Block Settings > Styles). Apply the variation and save the changes. Verify the contents are the expected (background color is aliceblue) — also in the frontend.

Screenshots or screencast

@ramonjd
Synching WordPress/wordpress-develop#6843
0d79048 
Cache reading theme.json files from styles/ folder.
@ramonjd ramonjd added the Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core label Jun 18, 2024
@ramonjd ramonjd requested a review from spacedmonkey as a code owner June 18, 2024 02:47
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 18, 2024 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions GitHub Actions
Copy link

Flaky tests detected in 0d79048.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9558154045
📝 Reported issues:

@ramonjd ramonjd self-assigned this Jun 18, 2024
@ramonjd ramonjd mentioned this pull request Jun 18, 2024
Closed
aaronrobertshaw
aaronrobertshaw approved these changes Jun 18, 2024
View reviewed changes
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Tests as advertised.

✅ Theme style variations are available as before
✅ Block style variations are also correct
✅ Works with child theme definitions

Theme Style Variations Block Style Variations
Screenshot 2024-06-18 at 2 32 42 PM Screenshot 2024-06-18 at 2 32 51 PM

@ramonjd
Copy link
Member Author

ramonjd commented Jun 18, 2024

From WordPress/wordpress-develop#6843 (review)

I wonder if we can also avoid the potential of processing the same decoded files multiple times as well?

I briefly played around with caching decoded, processed variation files in a local class property.

I took "processed" to mean:

  • run through static::translate, then
  • using the output to generate a new WP_Theme_JSON_Gutenberg object

Pretty naive implementation, by using the variation file path as a key, similar to theme_json_file_cache.

Seems to work with very limited testing, but probably deserves its own PR with test cases. Also, the question of whether to extend caching to other theme data. Caching is hard. 😄

@ramonjd ramonjd merged commit f45aa20 into trunk Jun 18, 2024
69 checks passed
@ramonjd ramonjd deleted the update/cache-reading-files-from-filesystem branch June 18, 2024 04:50
@github-actions github-actions bot added this to the Gutenberg 18.7 milestone Jun 18, 2024
@aaronrobertshaw
Copy link
Contributor

I briefly played around with caching decoded, processed variation files in a local class property.

Sounds similar to the original PR: #62610

I'm sure we can narrow down a solution. This is a good first step though.

@aaronrobertshaw aaronrobertshaw mentioned this pull request Jun 19, 2024
Open
@kevin940726 kevin940726 added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. CSS Styling Related to editor and front end styles, CSS-specific issues. and removed [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core labels Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronrobertshaw aaronrobertshaw aaronrobertshaw approved these changes

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey spacedmonkey is a code owner

@oandregal oandregal Awaiting requested review from oandregal

Assignees

@ramonjd ramonjd

Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Milestone
Gutenberg 18.7
Development

Successfully merging this pull request may close these issues.
3 participants
@ramonjd @aaronrobertshaw @kevin940726